check for __has_attribute(x) and/or clang instead of only basing existence on ZEND_GCC_VERSION#21622
check for __has_attribute(x) and/or clang instead of only basing existence on ZEND_GCC_VERSION#21622henderkes wants to merge 5 commits intophp:masterfrom
Conversation
139ad97 to
99c5aa3
Compare
ext/opcache/zend_shared_alloc.h
Outdated
| } align_test; | ||
|
|
||
| #if ZEND_GCC_VERSION >= 2000 | ||
| #if ZEND_GCC_VERSION >= 2000 || defined(__clang__) |
There was a problem hiding this comment.
Possibly for a follow-up, but we could define C23's alignof in zend_portability.h, and then use it here:
#if !defined(alignof)
# if ZEND_GCC_VERSION >= 2000 || defined(__clang__)
# define alignof(x) __alignof__(x)
# endif
#endif
There was a problem hiding this comment.
What if we use MSVC to compile and aren't in C23 mode? It won't be defined at all, so we couldn't use it directly. I think that's for a follow up PR.
There was a problem hiding this comment.
Yes you would still need an #ifdef alignof
There was a problem hiding this comment.
Thinking about this, I don't think it makes sense. __alignof__ and alignof don't function the same way. We can however use alignof here if it exists.
There was a problem hiding this comment.
#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L
# define PLATFORM_ALIGNMENT (alignof(align_test) < 8 ? 8 : alignof(align_test))
#elif defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L
# define PLATFORM_ALIGNMENT (_Alignof(align_test) < 8 ? 8 : _Alignof(align_test))
#elif ZEND_GCC_VERSION >= 2000 || defined(__clang__)
# define PLATFORM_ALIGNMENT (__alignof__(align_test) < 8 ? 8 : __alignof__(align_test))
#else
# define PLATFORM_ALIGNMENT (sizeof(align_test))
#endifwe could do this? It's a bit silly, but I don't want to use deprecated _Alignof in C23+
There was a problem hiding this comment.
Thinking about this, I don't think it makes sense. alignof and alignof don't function the same way. We can however use alignof here if it exists.
Good point, I wasn't aware of this
#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L # define PLATFORM_ALIGNMENT (alignof(align_test) < 8 ? 8 : alignof(align_test)) #elif defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L # define PLATFORM_ALIGNMENT (_Alignof(align_test) < 8 ? 8 : _Alignof(align_test)) #elif ZEND_GCC_VERSION >= 2000 || defined(__clang__) # define PLATFORM_ALIGNMENT (__alignof__(align_test) < 8 ? 8 : __alignof__(align_test)) #else # define PLATFORM_ALIGNMENT (sizeof(align_test)) #endifwe could do this? It's a bit silly, but I don't want to use deprecated _Alignof in C23+
Yes, given that the macro already had a fallback to sizeof(), this looks good to me 👍
…plicate lengthy #if checks
8c08131 to
4341ce5
Compare
initially found this for ZEND_COLD and then took a quick ctrl + shift + f for
ZEND_GCC_VERSION >=